Skip to content
This repository has been archived by the owner on Sep 5, 2023. It is now read-only.

common: move on to the new coverage uploader #2126

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ldorau
Copy link
Member

@ldorau ldorau commented Mar 14, 2023

Move on to the new coverage uploader.
The old bash uploader is long EOL now.

See: pmem/pmdk#5548


This change is Reviewable

@ldorau ldorau force-pushed the common-move-on-to-the-new-coverage-uploader branch from 619389a to 3e89b0b Compare March 14, 2023 12:02
@ldorau ldorau marked this pull request as draft March 14, 2023 12:02
Copy link
Member Author

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion

a discussion (no related file):
Something is not working yet (unusable report): https://app.codecov.io/github/pmem/rpma/commit/3e89b0b4957d9831ced07684f04a2191b7f6a241


Copy link
Member

@lukaszstolarczuk lukaszstolarczuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 5 files reviewed, 4 unresolved discussions (waiting on @ldorau)


.github/workflows/coverage.yml line 30 at r1 (raw file):

    steps:
       - name: Clone the git repo
         uses: actions/checkout@v1

I believe there's v3 already


.github/workflows/coverage.yml line 40 at r1 (raw file):

       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v3
         with:

you can enable verbose log: verbose: true


utils/docker/build.sh line 74 at r1 (raw file):

docker run --privileged=true --name=$containerName -i $TTY \
	$DNS_SETTING \
	${docker_opts} \

redundant now, I believe

Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @ldorau and @lukaszstolarczuk)

@ldorau ldorau requested a review from grom72 March 15, 2023 14:49
Copy link
Member Author

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @grom72 and @lukaszstolarczuk)


.github/workflows/coverage.yml line 30 at r1 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

I believe there's v3 already

Done.

Code quote:

uses: actions/checkout@v1

.github/workflows/coverage.yml line 40 at r1 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

you can enable verbose log: verbose: true

Done


utils/docker/build.sh line 74 at r1 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

redundant now, I believe

Right.

Code quote:

${docker_opts}

@ldorau ldorau force-pushed the common-move-on-to-the-new-coverage-uploader branch from 3e89b0b to 894cc09 Compare March 15, 2023 14:49
Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @lukaszstolarczuk)

@ldorau ldorau force-pushed the common-move-on-to-the-new-coverage-uploader branch from 894cc09 to d132e58 Compare March 15, 2023 18:34
Copy link
Contributor

@yangx-jy yangx-jy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 4 of 5 files reviewed, 7 unresolved discussions (waiting on @grom72, @ldorau, and @lukaszstolarczuk)


.github/workflows/coverage.yml line 33 at r3 (raw file):

       - name: Pull or rebuild the image
         run: cd $WORKDIR && ${{ matrix.CONFIG }} ./pull-or-rebuild-image.sh

You may need to use the following definition for ${{ matrix.CONFIG }}:

      matrix:
        CONFIG: ["N=1 OS=ubuntu OS_VER=latest TYPE=normal CC=gcc" TESTS_COVERAGE=1]

or remove ${{ matrix.CONFIG }} directly.


.github/workflows/coverage.yml line 36 at r3 (raw file):

       - name: Run the build
         run: cd $WORKDIR && ${{ matrix.CONFIG }} ./build.sh

The same comment.


.github/workflows/coverage.yml line 39 at r3 (raw file):

       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v3

Is download-scripts.sh still necessary if we plan to use download-scripts.sh?

@yangx-jy
Copy link
Contributor

.github/workflows/coverage.yml line 39 at r3 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

Is download-scripts.sh still necessary if we plan to use download-scripts.sh?

Sorry, I mean that we plan to use codecov/codecov-action@v3.

Code quote:

codecov/codecov-action@v3

@ldorau ldorau force-pushed the common-move-on-to-the-new-coverage-uploader branch from d132e58 to ac0240e Compare March 16, 2023 07:58
@ldorau ldorau requested a review from grom72 March 16, 2023 07:58
Copy link
Member Author

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 4 of 5 files reviewed, 7 unresolved discussions (waiting on @grom72, @lukaszstolarczuk, and @yangx-jy)


.github/workflows/coverage.yml line 33 at r3 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

You may need to use the following definition for ${{ matrix.CONFIG }}:

      matrix:
        CONFIG: ["N=1 OS=ubuntu OS_VER=latest TYPE=normal CC=gcc" TESTS_COVERAGE=1]

or remove ${{ matrix.CONFIG }} directly.

It's just a missed leftover. I removed the build matrix on purpose and just forgot to remove it.

Removed.


.github/workflows/coverage.yml line 36 at r3 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

The same comment.

Done.


.github/workflows/coverage.yml line 39 at r3 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

Sorry, I mean that we plan to use codecov/codecov-action@v3.

Right, they will not be needed. Removed.

Copy link
Member

@lukaszstolarczuk lukaszstolarczuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 5 files at r1, 1 of 2 files at r2, 1 of 1 files at r3, 5 of 5 files at r4, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @grom72 and @yangx-jy)

Copy link
Contributor

@yangx-jy yangx-jy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72)

Copy link
Member Author

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72)

a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

Something is not working yet: https://app.codecov.io/github/pmem/rpma/commit/3e89b0b4957d9831ced07684f04a2191b7f6a241

The last result (still "unusable report"): https://app.codecov.io/github/pmem/rpma/commit/ac0240e7d421d64310f4d0106f30432e886c5e59


@lukaszstolarczuk
Copy link
Member

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72)

a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…
The last result (still "unusable report"): https://app.codecov.io/github/pmem/rpma/commit/ac0240e7d421d64310f4d0106f30432e886c5e59

if you believe you checked everything and are convinced you're doing everything right - you can ask Codecov team for help. They were usually very helpful on their forum.

Copy link
Member Author

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72 and @lukaszstolarczuk)

a discussion (no related file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

if you believe you checked everything and are convinced you're doing everything right - you can ask Codecov team for help. They were usually very helpful on their forum.

Let's see: https://community.codecov.com/t/unusable-report-using-codecov-action-v3-bash-uploader-works-well/4223


Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r3, 5 of 5 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukaszstolarczuk)

Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukaszstolarczuk)

Copy link
Member Author

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukaszstolarczuk)

a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

Let's see: https://community.codecov.com/t/unusable-report-using-codecov-action-v3-bash-uploader-works-well/4223

Waiting for help from the support team: https://community.codecov.com/t/unusable-report-using-codecov-action-v3-bash-uploader-works-well/4223 ...


@ldorau ldorau force-pushed the common-move-on-to-the-new-coverage-uploader branch from ac0240e to 333b79b Compare April 14, 2023 13:33
Move on to the new coverage uploader.
The old bash uploader is long EOL now.

See: pmem/pmdk#5548
@ldorau ldorau force-pushed the common-move-on-to-the-new-coverage-uploader branch from 333b79b to 2660e6b Compare April 14, 2023 13:47
Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukaszstolarczuk)

Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ldorau and @lukaszstolarczuk)

a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

Waiting for help from the support team: https://community.codecov.com/t/unusable-report-using-codecov-action-v3-bash-uploader-works-well/4223 ...

@ldorau, does it work now as expected?


@ldorau ldorau requested a review from grom72 April 17, 2023 07:37
Copy link
Member Author

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72 and @lukaszstolarczuk)

a discussion (no related file):

Previously, grom72 (Tomasz Gromadzki) wrote…

@ldorau, does it work now as expected?

No, it still does not work


@yangx-jy
Copy link
Contributor

yangx-jy commented May 8, 2023

Previously, ldorau (Lukasz Dorau) wrote…

No, it still does not work

Is there any progress on this PR? ^_^

Copy link
Member Author

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @grom72)

a discussion (no related file):

Previously, yangx-jy (Xiao Yang) wrote…

Is there any progress on this PR? ^_^

The codecov/codecov-action@v3 action still does not work as expected:
https://community.codecov.com/t/unusable-report-using-codecov-action-v3-bash-uploader-works-well/4223/17


@yangx-jy
Copy link
Contributor

Previously, ldorau (Lukasz Dorau) wrote…

The codecov/codecov-action@v3 action still does not work as expected:
https://community.codecov.com/t/unusable-report-using-codecov-action-v3-bash-uploader-works-well/4223/17

It seems hard to make a progress on the issue. Do we have any other ideas? When do you prepare to do the release?

Copy link
Member Author

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @grom72)

a discussion (no related file):

Previously, yangx-jy (Xiao Yang) wrote…

It seems hard to make a progress on the issue. Do we have any other ideas? When do you prepare to do the release?

v1.3.0 released: https://github.com/pmem/rpma/releases/tag/1.3.0


@yangx-jy
Copy link
Contributor

Previously, ldorau (Lukasz Dorau) wrote…

v1.3.0 released: https://github.com/pmem/rpma/releases/tag/1.3.0

@ldorau Great. Thanks a lot for your help.

Copy link
Member Author

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72, @lukaszstolarczuk, and @yangx-jy)

a discussion (no related file):

Previously, yangx-jy (Xiao Yang) wrote…

@ldorau Great. Thanks a lot for your help.

You are welcome :-)


Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants